Skip to content

ci: test qjson against OpenResty runtimes#99

Merged
membphis merged 2 commits into
mainfrom
codex/issue-97-openresty-runtime-matrix
May 30, 2026
Merged

ci: test qjson against OpenResty runtimes#99
membphis merged 2 commits into
mainfrom
codex/issue-97-openresty-runtime-matrix

Conversation

@membphis
Copy link
Copy Markdown
Collaborator

@membphis membphis commented May 30, 2026

Closes #97

Summary

  • add a PR CI OpenResty runtime matrix for 1.21.4.4, 1.27.1.2, and 1.29.2.4 official Jammy Docker images
  • run a small resty smoke script covering require("qjson"), parse/get accessors, lazy decode, encode, and GC after dropping values
  • run the existing Lua busted suite with each OpenResty bundle's own luajit binary and explicit LD_LIBRARY_PATH for ffi.load("qjson")

Tests

  • bash -n .github/scripts/run-openresty-runtime.sh
  • ruby -e 'require "yaml"; YAML.load_file(".github/workflows/ci.yml"); puts "yaml ok"'
  • go run github.com/rhysd/actionlint/cmd/actionlint@latest .github/workflows/ci.yml
  • DYLD_LIBRARY_PATH="$PWD/target/release" LUA_PATH='./lua/?.lua;;' luajit .github/scripts/openresty-smoke.lua
  • make test
  • make lint
  • cargo test --release --no-default-features
  • cargo test --features test-panic --release
  • OPENRESTY_IMAGE=openresty/openresty:1.21.4.4-0-jammy .github/scripts/run-openresty-runtime.sh after building a Linux cdylib inside the same OpenResty Jammy Docker runtime

Summary by CodeRabbit

  • Tests
    • Added comprehensive smoke tests for OpenResty/Lua runtime integration, validating parsing, data retrieval, null detection, and encoding operations.
    • Expanded CI pipeline with automated testing across multiple OpenResty versions in containerized environments to ensure runtime compatibility.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 30, 2026

Warning

Review limit reached

@membphis, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 3 minutes and 30 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4e55ae53-86a6-4498-8224-d2fce838e3c5

📥 Commits

Reviewing files that changed from the base of the PR and between 0e0597b and 4fa9a75.

📒 Files selected for processing (1)
  • .github/workflows/ci.yml
📝 Walkthrough

Walkthrough

This PR adds OpenResty runtime testing infrastructure to verify qjson FFI behavior across multiple OpenResty versions. It introduces a Lua smoke test, a Docker-based runtime execution script, and a GitHub Actions job that runs this verification matrix on each pull request.

Changes

OpenResty Runtime Verification

Layer / File(s) Summary
Smoke test and runtime orchestration
.github/scripts/openresty-smoke.lua, .github/scripts/run-openresty-runtime.sh
Lua smoke test validates qjson parsing, typed getters, null detection, nested structure decoding, and JSON encoding with round-trip assertions. Bash script checks for compiled libqjson.so, runs Docker with workspace bind-mount, installs dependencies, configures library and Lua module paths, and executes the smoke test and busted test suite under the container's LuaJIT.
CI workflow and version matrix
.github/workflows/ci.yml
New openresty GitHub Actions job runs after Rust job on ubuntu-22.04 across a matrix of OpenResty versions. Job installs Rust toolchain, caches Cargo artifacts, builds release cdylib, and runs runtime verification with OPENRESTY_IMAGE set from matrix variable.

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
E2e Test Quality Review ⚠️ Warning Line 42: test -x "$BUSTED" fails; busted installed to /usr/bin, not $OPENRESTY/luajit/bin. Line 49: resty missing -I /workspace/lua; LUA_PATH env var not used by resty, preventing require("qjson"). Line 25: BUSTED=busted; Line 42: command -v busted >/dev/null; Line 49: "$RESTY" -I /workspace/lua /workspace/.github/scripts/openresty-smoke.lua
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'ci: test qjson against OpenResty runtimes' accurately summarizes the main change—adding OpenResty runtime testing to CI.
Linked Issues check ✅ Passed The PR fully implements issue #97 objectives: builds qjson in release mode, tests against OpenResty runtime matrix (1.21.4.4, 1.27.1.2, 1.29.2.4), runs smoke script and busted suite with bundled LuaJIT, and ensures ffi.load resolves the release cdylib.
Out of Scope Changes check ✅ Passed All changes are scoped to CI and runtime testing as specified: OpenResty smoke script, runtime wrapper script, and CI workflow. No unrelated functionality or scope creep detected.
Security Check ✅ Passed No security vulnerabilities found across all 7 categories: no credential exposure, no TLS errors, no auth bypasses, variables properly handled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/issue-97-openresty-runtime-matrix

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)

299-302: ⚡ Quick win

Consider scoping least-privilege permissions for the new job.

The workflow has no permissions: block, so the job runs with the default (broad) GITHUB_TOKEN. This job only checks out code and runs containers, so contents: read is sufficient and reduces token exposure.

🔒 Proposed addition (job-level)
   openresty:
     name: OpenResty runtime (${{ matrix.openresty.version }})
     runs-on: ubuntu-22.04
     needs: rust
+    permissions:
+      contents: read
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/ci.yml around lines 299 - 302, The openresty job is
missing a permissions block and therefore gets the default broad GITHUB_TOKEN;
add a job-level permissions entry for the openresty job (reference job name
"openresty") and set at minimum permissions: contents: read to enforce
least-privilege for this job that only checks out code and runs containers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/scripts/run-openresty-runtime.sh:
- Around line 46-49: The resty invocation uses the RESTY variable to run
openresty-smoke.lua but relies on LUA_PATH which resty-cli doesn't apply; update
the shell invocation that runs openresty-smoke.lua (the "$RESTY" call) to pass
the Lua include directory with resty's -I/--lua-path flag (pointing at
/workspace/lua) so require("qjson") can resolve, or alternatively add code in
openresty-smoke.lua to prepend "/workspace/lua/?.lua" to package.path before
requiring qjson.
- Line 25: The script sets BUSTED="$OPENRESTY/luajit/bin/busted" which is
incorrect for how busted is installed and causes test -x "$BUSTED" to fail;
update run-openresty-runtime.sh to locate busted from the PATH (e.g., use
command -v busted or fallback to /usr/bin/busted) and change the existence check
to use that discovered path (symbol: BUSTED and the test -x "$BUSTED"). Also
ensure resty can find lua/qjson.lua when running openresty-smoke.lua by adding
the include flag to resty (use resty -I /workspace/lua ...) or set nginx
lua_package_path accordingly instead of relying on the LUA_PATH environment
variable (symbols: resty, openresty-smoke.lua, LUA_PATH).

---

Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 299-302: The openresty job is missing a permissions block and
therefore gets the default broad GITHUB_TOKEN; add a job-level permissions entry
for the openresty job (reference job name "openresty") and set at minimum
permissions: contents: read to enforce least-privilege for this job that only
checks out code and runs containers.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 462b0732-a552-4a22-9d21-5ad88b8352dc

📥 Commits

Reviewing files that changed from the base of the PR and between 3cc1d32 and 0e0597b.

📒 Files selected for processing (3)
  • .github/scripts/openresty-smoke.lua
  • .github/scripts/run-openresty-runtime.sh
  • .github/workflows/ci.yml

OPENRESTY=/usr/local/openresty
RESTY="$OPENRESTY/bin/resty"
LUAJIT="$OPENRESTY/luajit/bin/luajit"
BUSTED="$OPENRESTY/luajit/bin/busted"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate and show the relevant portion of the OpenResty runtime script
FILE=".github/scripts/run-openresty-runtime.sh"
test -f "$FILE" || { echo "Missing $FILE"; exit 1; }

echo "=== $FILE (head/tail around suspected lines) ==="
nl -ba "$FILE" | sed -n '1,120p'

echo
echo "=== Search for busted references in the repo ==="
rg -n --hidden --no-ignore-vcs "BUSTED|busted" .github || true

echo
echo "=== Search workflows/jobs that run 'busted' ==="
rg -n --hidden --no-ignore-vcs "run-openresty-runtime\.sh|openresty-runtime|busted" .github/workflows || true

Repository: api7/lua-qjson

Length of output: 203


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE=".github/scripts/run-openresty-runtime.sh"
test -f "$FILE" || { echo "Missing $FILE"; exit 1; }

echo "=== $FILE (first 120 lines) ==="
awk '{printf "%d:%s\n", NR, $0}' "$FILE" | sed -n '1,120p'

echo
echo "=== $FILE (lines 20-80) ==="
awk '{printf "%d:%s\n", NR, $0}' "$FILE" | sed -n '20,80p'

echo
echo "=== Search for busted references in the repo ==="
rg -n --hidden --no-ignore-vcs "BUSTED|busted" .github || true

echo
echo "=== Search workflows/jobs that run this script ==="
rg -n --hidden --no-ignore-vcs "run-openresty-runtime\.sh|openresty-runtime" .github/workflows || true

echo
echo "=== Show any job step that runs busted directly (workflows) ==="
rg -n --hidden --no-ignore-vcs "busted" .github/workflows || true

Repository: api7/lua-qjson

Length of output: 4426


🏁 Script executed:

#!/bin/bash
set -euo pipefail
FILE=".github/workflows/ci.yml"
test -f "$FILE"
awk 'NR>=150 && NR<=420 {print NR ":" $0}' "$FILE" | sed -n '1,350p'

Repository: api7/lua-qjson

Length of output: 10140


🏁 Script executed:

#!/bin/bash
set -euo pipefail
FILE=".github/workflows/ci.yml"
test -f "$FILE"
awk 'NR>=150 && NR<=420 {print NR ":" $0}' "$FILE" | sed -n '1,350p'

Repository: api7/lua-qjson

Length of output: 10140


🌐 Web query:

OpenResty Docker image /usr/local/openresty/luajit/bin/busted exists

💡 Result:

In the official OpenResty Docker images (openresty/openresty), the file path /usr/local/openresty/luajit/bin/busted is not something the image provides by default. What the image does provide by default is LuaJIT/luarocks at /usr/local/openresty/luajit/bin/luarocks (as documented in the docker-openresty repo README) [1]. The OpenResty image Dockerfile builds and installs LuaRocks, but it does not document installing the busted test runner into /usr/local/openresty/luajit/bin/busted [2]. Therefore, you should expect that /usr/local/openresty/luajit/bin/busted will be missing unless you install busted yourself (e.g., via luarocks) in your own Dockerfile or at container build time. If you specifically want, tell me which OpenResty image tag you’re using (e.g., openresty/openresty:1.27.1.2-0-bookworm-fat), and I can help craft the exact Dockerfile snippet to install busted in that image.

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail
FILE=".github/scripts/openresty-smoke.lua"
test -f "$FILE" && awk '{printf "%d:%s\n", NR, $0}' "$FILE" | sed -n '1,200p' || echo "Missing $FILE"

Repository: api7/lua-qjson

Length of output: 1018


🏁 Script executed:

#!/bin/bash
set -euo pipefail
FILE=".github/scripts/openresty-smoke.lua"
test -f "$FILE" || { echo "Missing $FILE"; exit 1; }
awk '{printf "%d:%s\n", NR, $0}' "$FILE" | sed -n '1,200p'

Repository: api7/lua-qjson

Length of output: 1018


🌐 Web query:

OpenResty resty command line LUA_PATH environment variable honored require package.path

💡 Result:

The OpenResty resty command does not automatically honor the shell environment variable LUA_PATH for Lua module resolution; instead it uses OpenResty/Lua’s lua_package_path/package.path setup that can be influenced via resty’s own CLI options (notably -I) and the nginx/OpenResty configuration it generates. What the resty CLI provides (authoritative): 1) resty has an -I DIR option specifically “Add dir to the search paths for Lua libraries.” [1][2] 2) The official demo shows resty -I lua/ -e 'require "test".hello()' and states: “Without the -I option, it won’t find it. This is because the lua/ directory is not in the Lua module search paths by default.” [3] 3) In the resty implementation, -I arguments are parsed and stored as include directories for Lua library search paths. [1] Implication for LUA_PATH: - Since the documented and implemented mechanism for adding Lua search locations with resty is -I (not LUA_PATH), and the demo explicitly frames the problem as “directory not in the Lua module search paths by default” unless you use -I, you should not rely on LUA_PATH being honored by resty’s require behavior. [1][3] Practical ways to make require work: A) Use resty -I: - Example pattern: resty -I /path/to/lua -e 'require "your.module"' [3] B) Or adjust OpenResty/Lua’s lua_package_path (nginx-side), since that’s what ultimately determines Lua’s package.path used by require: - In OpenResty nginx Lua module docs, lua_package_path is the directive that “set[s] search paths for pure Lua external libraries”. [4] Notes about “it sometimes seems to work” in containers: - There are reports that in some OpenResty Docker image variants, the container defines a LUA_PATH value and require may work when that path is consistent with the image’s configured Lua search paths. But this is image-specific behavior, not an advertised/guaranteed contract that resty honors LUA_PATH directly. [5]

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail
ls -la
test -f "lua/qjson.lua" && echo "FOUND lua/qjson.lua" || echo "MISSING lua/qjson.lua"
test -f ".github/scripts/openresty-smoke.lua" && sed -n '1,120p' .github/scripts/openresty-smoke.lua

Repository: api7/lua-qjson

Length of output: 2557


Fix busted lookup in run-openresty-runtime.sh (and make resty find lua/qjson.lua)

  • run-openresty-runtime.sh sets BUSTED="$OPENRESTY/luajit/bin/busted" but installs busted via /usr/bin/luarocks install busted; the OpenResty image doesn’t provide /usr/local/openresty/luajit/bin/busted by default, so test -x "$BUSTED" will fail under set -euo pipefail.
  • openresty-smoke.lua does require("qjson"), and run-openresty-runtime.sh relies on export LUA_PATH="/workspace/lua/?.lua;;" for that, but resty’s Lua search paths are driven by resty -I ... (not LUA_PATH env). Add -I /workspace/lua (or set nginx lua_package_path) to ensure lua/qjson.lua is found.
🐛 Proposed fix (for `busted`)
-BUSTED="$OPENRESTY/luajit/bin/busted"
+# busted is installed below via the Ubuntu LuaRocks tree and resolved from PATH.
+BUSTED=busted
-test -x "$BUSTED"
+command -v "$BUSTED" >/dev/null
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/scripts/run-openresty-runtime.sh at line 25, The script sets
BUSTED="$OPENRESTY/luajit/bin/busted" which is incorrect for how busted is
installed and causes test -x "$BUSTED" to fail; update run-openresty-runtime.sh
to locate busted from the PATH (e.g., use command -v busted or fallback to
/usr/bin/busted) and change the existence check to use that discovered path
(symbol: BUSTED and the test -x "$BUSTED"). Also ensure resty can find
lua/qjson.lua when running openresty-smoke.lua by adding the include flag to
resty (use resty -I /workspace/lua ...) or set nginx lua_package_path
accordingly instead of relying on the LUA_PATH environment variable (symbols:
resty, openresty-smoke.lua, LUA_PATH).

Comment on lines +46 to +49
export LD_LIBRARY_PATH=/workspace/target/release
export LUA_PATH="/workspace/lua/?.lua;;"

"$RESTY" /workspace/.github/scripts/openresty-smoke.lua
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

Does the OpenResty resty CLI respect the LUA_PATH environment variable for package.path, or is --lua-path / -I required?

💡 Result:

The resty (OpenResty resty-cli) command does not appear to automatically honor the LUA_PATH environment variable to set Lua’s package.path; instead, you should use the resty CLI’s Lua path option (-I/--lua-path depending on version) to add search directories to Lua’s module search paths. Evidence: resty-cli documents that “-I DIR Add dir to the search paths for Lua libraries” [1][2], and its implementation builds a lua_package_path string from the provided -I directories (it generates lua_package_path "…"; and lua_package_cpath "…";) rather than reading LUA_PATH [3]. The generated nginx config snippet explicitly sets lua_package_path based on those include directories [3]. So, to ensure your Lua modules are found when running with resty, use -I/--lua-path (or the equivalent path-adding flag provided by your resty version) rather than relying on LUA_PATH being applied to package.path automatically.

Citations:


Pass Lua module include paths to resty instead of relying on LUA_PATH
openresty-smoke.lua does require("qjson"), but run-openresty-runtime.sh (lines 46-49) only sets LUA_PATH and invokes "$RESTY" /workspace/.github/scripts/openresty-smoke.lua without -I/--lua-path. resty-cli uses its own Lua path option(s) to build lua_package_path (it doesn’t automatically apply LUA_PATH), so /workspace/lua/qjson.lua may not be found. Add the appropriate resty -I/--lua-path flag (or set package.path inside the smoke script).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/scripts/run-openresty-runtime.sh around lines 46 - 49, The resty
invocation uses the RESTY variable to run openresty-smoke.lua but relies on
LUA_PATH which resty-cli doesn't apply; update the shell invocation that runs
openresty-smoke.lua (the "$RESTY" call) to pass the Lua include directory with
resty's -I/--lua-path flag (pointing at /workspace/lua) so require("qjson") can
resolve, or alternatively add code in openresty-smoke.lua to prepend
"/workspace/lua/?.lua" to package.path before requiring qjson.

@membphis membphis merged commit 6fc5da1 into main May 30, 2026
15 checks passed
@membphis membphis deleted the codex/issue-97-openresty-runtime-matrix branch May 30, 2026 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ci: test qjson against multiple OpenResty runtimes

1 participant